Replace request with axios & Upgrade CI to NodeJS 20#331
Replace request with axios & Upgrade CI to NodeJS 20#331florian-h05 merged 6 commits intoopenhab:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to resolve CI failures caused by a Node.js version mismatch by moving CI from Node 10.x to Node 18.x and aligning repository configuration with that requirement. In addition to the CI workflow update, it introduces local install-time enforcement/pinning of Node 18 via a new preinstall hook and Volta configuration.
Changes:
- Update GitHub Actions CI to use Node.js 18.x for build/package jobs.
- Add a repository-level
preinstallscript that checks Node compatibility and guides (or attempts) Volta setup. - Pin Node 18.20.4 via Volta and adjust related
package.jsonscripts/dependencies.
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
.github/workflows/ci.yml |
Switch CI Node version to 18.x (also includes whitespace changes in triggers). |
scripts/preinstall.js |
New preinstall-time Node/Volta compatibility logic with optional auto-install behavior. |
package.json |
Adds engines.node, Volta pinning, preinstall/pretest-compile scripts, and updates webpack-cli. |
.gitignore |
Adds Copilot-related ignore patterns (including a specific archived session file). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The CI workflow was configured to use Node.js 10.x, but the project now requires Node.js 18.x (as specified in package.json). This mismatch caused CI failures on all platforms: - macOS: Unable to find Node version '10.x' for arm64 - Ubuntu/Package: Preinstall script detected version mismatch and npm install failed - Azure: Build failed due to missing dependencies docs: add session report for PR openhab#331 review and fixes chore(ci): update actions/stale from v3.0.18 to v9 Update the stale issues workflow action to use the currently maintained major version (v9) instead of the deprecated v3.0.18. This ensures we receive the latest security patches and runtime fixes. feat: implement Volta configuration with dynamic Node version - Read Node version requirement from package.json engines.node field - Create configureVoltaWith() function to execute volta pin node@<version> - Integrate Volta configuration into main flow when Volta is detected - Replace hardcoded version references with dynamic VOLTA_VERSION_REQUIRED - Add graceful error handling with non-blocking warnings - Update header comment to reflect configuration behavior - Add packageJsonPath constant for version source - Add readNodeVersionFromPackageJson() helper function This resolves the inconsistency where the script header claimed to 'configure Volta' but only validated Node compatibility. The script now actively pins the Node version via volta pin when Volta is detected and managing the current Node process. Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 11 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a969a38 to
edb3944
Compare
- TypeScript: 3.9.7 → 4.9.5 (old version incompatible with Node 18) - ts-loader: 6.2.2 → 9.5.0 (required for TypeScript 4.x) - @types/lodash: 4.14.167 → 4.14.195 (fixes corrupted type definitions) - @types/node: 8.10.66 → 18.18.0 (Node 18 type definitions) Resolves PR openhab#331: Node.js version upgrade from 10.x to 18.x Fixes 94 TypeScript compilation errors related to lodash type definitions Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
The CI workflow was configured to use Node.js 10.x, but the project now requires Node.js 18.x (as specified in package.json). This mismatch caused CI failures on all platforms: - macOS: Unable to find Node version '10.x' for arm64 - Ubuntu/Package: Preinstall script detected version mismatch and npm install failed - Azure: Build failed due to missing dependencies docs: add session report for PR openhab#331 review and fixes chore(ci): update actions/stale from v3.0.18 to v9 Update the stale issues workflow action to use the currently maintained major version (v9) instead of the deprecated v3.0.18. This ensures we receive the latest security patches and runtime fixes. feat: implement Volta configuration with dynamic Node version - Read Node version requirement from package.json engines.node field - Create configureVoltaWith() function to execute volta pin node@<version> - Integrate Volta configuration into main flow when Volta is detected - Replace hardcoded version references with dynamic VOLTA_VERSION_REQUIRED - Add graceful error handling with non-blocking warnings - Update header comment to reflect configuration behavior - Add packageJsonPath constant for version source - Add readNodeVersionFromPackageJson() helper function This resolves the inconsistency where the script header claimed to 'configure Volta' but only validated Node compatibility. The script now actively pins the Node version via volta pin when Volta is detected and managing the current Node process. Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
- TypeScript: 3.9.7 → 4.9.5 (old version incompatible with Node 18) - ts-loader: 6.2.2 → 9.5.0 (required for TypeScript 4.x) - @types/lodash: 4.14.167 → 4.14.195 (fixes corrupted type definitions) - @types/node: 8.10.66 → 18.18.0 (Node 18 type definitions) Resolves PR openhab#331: Node.js version upgrade from 10.x to 18.x Fixes 94 TypeScript compilation errors related to lodash type definitions Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
This commit consolidates the following improvements: - Upgrade Node.js minimum version from 10.x to 20.x LTS for security and performance - Update CI/CD pipeline to use Node.js 20.x and latest GitHub Actions versions - Add Volta version pinning for consistent development environment (20.x) - Add .nvmrc for Node version management - Implement Node.js compatibility verification in preinstall script - Update deprecated GitHub Actions (checkout v4, setup-node v4, stale-issues v9) - Fix CI paths-ignore patterns for .azure-pipelines directory - Update TypeScript and related dependencies for Node.js 20 compatibility - Modernize package.json engines field with exact version constraints - Improve ItemCompletionProvider test coverage and error handling - Update Volta configuration for automatic Node.js management Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
|
@openhab/maintainers & @s0170071 Please review ➡ some modernization to build the basis for future development. The build installs ok, and basics functions look well:
|
…ion logic - Clarify script purpose: Node version validation and user guidance only - Remove unused VERSION_MANAGER_CONFIGS constant, replace with individual path variables - Enhance Volta verification to robustly check if current node is Volta-managed - Improve step-by-step comments in Volta detection flow - Update error messages to better distinguish failure modes - Clarify that script does not auto-install Volta (manual or opt-in only) Addresses PR review feedback on preinstall.js implementation clarity and robustness. Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
The script mentions setting VOLTA_INSTALL=1 for 'automatic installation', but this environment variable is never read or used. This creates false expectations and is misleading guidance. Since the script only provides manual installation guidance (not auto-install), remove the misleading VOLTA_INSTALL reference to be accurate and transparent about what the script actually does. Addresses PR comment openhab#16: misleading auto-install guidance. Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
The project requires Node >= 20 in engines field but was using @types/node @18. This mismatch causes TypeScript to type-check against outdated APIs. Update @types/node from ^18.18.0 to ^20.0.0 to ensure type definitions match the minimum required Node version. Addresses florian-h05 comment: 'Why Node 18 types if on Node >= 20?' Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
…iption - Move all changes from [1.0.1-alpha.1] section to [Unreleased] since version 1.0.1-alpha.1 has not been released yet - Fix GitHub Actions description: remove incorrect 'to v4' claim (actions/stale is v9, not v4) and clarify which actions were updated - Update @types/node version in changelog from 18.18.0 to 20.0.0 to match the actual dependency update Addresses florian-h05 comments on CHANGELOG.md accuracy. Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
|
Hi Florian, thank you for your review - can you check if you are happy with the changes made. Close the comments, or let me know if additional adjustments are required. with kind regards, |
florian-h05
left a comment
There was a problem hiding this comment.
LGTM, thanks!
Now I need to get write access to this repo ...
|
@florian-h05 You have it. 😉 |
|
That was fast, thanks 🚀 |

Updates CI/CD pipeline and development environment to Node.js 20.x LTS.
This change: